Conversation
A strong claim is almost equivalent to claim, but deep copies volatile buffers to new raw_char buffers. A volatile buffer is one which cannot be held/claimed for long periods due to external dependencies (e.g., Accelio message buffers are volatile). Strong claim is now the unmarked case, so is performed automatically in the buffer::list copy constructor. Low-level operations should allow explicit volatile sharing, so copying a buffer::list:iterator or a buffer::ptr works as normal. For explicit sharing of volatile buffers, a new boolean is added to the public claim_* methods, and a buffer::list::share method is also added, to share an entire sequence. Signed-off-by: Vu Pham <vu@mellanox.com> Signed-off-by: Matt Benjamin <matt@linuxbox.com> Signed-off-by: Matt Benjamin <matt@cohortfs.com> Conflicts: src/include/buffer.h Signed-off-by: Matt Benjamin <matt@cohortfs.com> Conflicts: src/common/buffer.cc
This change restores volatile sharing semantics in the Message decode path, and also in the OSD write path for FileStore/FileJournal. This can be verified with a breakpoint set at the clone/COW case in buffer::ptr::strong_claim (currently buffer.cc:690). Signed-off-by: Matt Benjamin <matt@linuxbox.com> Signed-off-by: Matt Benjamin <matt@cohortfs.com>
Signed-off-by: Matt Benjamin <matt@cohortfs.com>
Signed-off-by: Matt Benjamin <matt@cohortfs.com>
|
I have a quick look at accelio source codes. It looks like you want to let memory buffer which is used by ibv_rq can be append to bufferlist and recycle by accelio? |
|
Yeah, I'm not sure that I have got your idea. It looks like you can inherit "Raw" class which can add registered memory to bufferlist and a special destruction which can recycle memory when ptr->nref == 0. |
|
Thanks for reviewing. First, yes, the Xio changesets do derive a "raw" class with special destruction. We've had ones that just recycle the memory, and a stronger one does so via the related completion hook. You're right, that's the natural way to deal with zero-copy buffers in the current buffer API (yuyuyu101). The issue turned out to be, not getting zero-copy to work, but ensuring that zero-copy buffers were returned in a timely manner. Because buffer::ptrs have been shared by default, we don't know when (or even if) every ref will be returned. So what strong claim is attempting to do is make sharing !default -for a specific class of buffers- (ones with the volatile property). I wanted a way to ensure that, unless a code path would assuredly return volatile buffers "in a timely manner" (something that seemed difficult to ensure "by default"), it would not get zero-copy buffers (an Accelio pool resource, whether or not they are registered). Sage, will that have the same effect? (I'm genuinely not sure.) |
|
liewegas, Even though strong_claim_inplace() is called by default for claim() family and operator=; however, in buffer::ptr::strong_claim() we check if raw buffer volatile or not to deep copy. By the default the raw buffer is not volatile. We only set is_volatile() == true in case of Accelio's buffers. |
|
Okay, this makes sense. Perhaps renaming strong_claim_* to strong_claim_volatile_* would make this clearer. Also, instead of a bool strong=true arg, what about a bool force_weak_volatile=false? That would make it clearer that the exceptional case specific to volatile buffers? |
Reserve last char in array for '\0' to ensure termination of the string. Fix for: CID 1128383 (#1 of 1): Buffer not null terminated (BUFFER_SIZE_WARNING) buffer_size_warning: Calling strncpy with a maximum size argument of 1000 bytes on destination array secret of size 1000 bytes might leave the destination string unterminated. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for another case of: CID 717112 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable io_ctx going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Release completion as soon as no longer needed. Fix for: CID 1219593 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable completion going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1054876 (#1 of 1): Use after free (USE_AFTER_FREE) pass_freed_arg: Passing freed pointer bucket_obj as an argument to put_bucket_obj Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Make sure k and m paramter are valid to prevent crash. Fix typo. Fix for the following CID and other possible invalid combinations of k/m parameter: CID 1219466 (#1 of 1): Division or modulo by zero (DIVIDE_BY_ZERO) divide_by_zero: In expression rand() % (k + m), modulo by expression k + m which may be zero has undefined behavior. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Check return value as done in all other places. Fix error messages to print correct function name getdir and not read_dir/readdir since the error isn't necessarily raised by read_dir(). Fix for: CID 1219463 (#1 of 1): Unchecked return value (CHECKED_RETURN) check_return: Calling getdir without checking return value (as is done elsewhere 4 out of 5 times). Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1054853 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking is_truncated suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Add vim line to file. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1128384 (#1 of 1): Ignoring number of bytes read (CHECKED_RETURN) check_return: fread(void * restrict, size_t, size_t, FILE * restrict) returns the number of bytes read, but it is ignored. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix memory leak caused by using std::string to hold result of strdup call returned from getObjName(). Fix for Coverity issues: CID 1221525 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Failing to save or free storage allocated by this->getObjName(soid, 0UL) leaks it. CID 1221526 (1-3 of 3): Resource leak (RESOURCE_LEAK) leaked_storage: Failing to save or free storage allocated by this->getObjName(soid, *) leaks it. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix indentation to the same as the original fsx.c . Fix for Coverity issue: CID 1219473 (#1-2 of 2): Nesting level does not match indentation (NESTING_INDENT_MISMATCH) uncle: This statement is indented to column 25, as if it were nested within the preceding parent statement, but it is not. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
In the checks to build only_in_b up the wrong const_iterator x is build up. it should compare rhs->xattrs with xattrs entries and not twice rhs->xattrs. Fix for: CID 716957 (#1 of 1): Invalid iterator comparison (MISMATCHED_ITERATOR) mismatched_comparison: Comparing x from rhs->xattrs to this->xattrs.end() from this->xattrs. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1128391 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this statement goto done_err; Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Wrong variable for return of rados_getxattrs_next() and rados_omap_get_next() used (len vs. val_len). Fix for: CID 1219464 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this expression key == NULL inside statement if (len == 0UL && key == NU... CID 1219465 (#1 of 1): Logically dead code (DEADCODE) dead_error_line: Execution cannot reach this expression key == NULL inside statement if (len == 0UL && key == NU... Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Close socket if set_nonblock() fails before return. Fix for: CID 1249633 (#1 of 1): Resource leak (RESOURCE_LEAK) 7. leaked_handle: Handle variable s going out of scope leaks the handle. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Close socket if ::setsockopt() fails before return. Fix for: CID 1249632 (#1 of 1): Resource leak (RESOURCE_LEAK) 9. leaked_handle: Handle variable s going out of scope leaks the handle. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1251453 (#2 of 2): Resource leak (RESOURCE_LEAK) leaked_storage: Variable io_ctx going out of scope leaks the storage it points to. CID 1251454 (#1 of 1): Resource leak (RESOURCE_LEAK) leaked_storage: Variable h going out of scope leaks the storage it points to. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Remove useless check for invalid parameter for "mds remove_data_pool", we have already the poolid from the correct parameter 'poolname'. Fix for: CID 1251445 (#1 of 1): Unchecked return value (CHECKED_RETURN) check_return: Calling cmd_getval without checking return value (is done elsewhere 19 out of 22 times). Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 717142 (#1 of 1): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::buffer::end_of_buffer is thrown and never caught. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 717141 (#1-19): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::FailedAssertion is thrown and never caught. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
CID 717157 (#1-2): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char const **) an exception of type ceph::FailedAssertion is thrown and never caught. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 717163 (#1 of 1): Uncaught exception (UNCAUGHT_EXCEPT) root_function: In function main(int, char **) an exception of type ceph::buffer::end_of_buffer is thrown and never caught. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1297860 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS) 1. Condition this->mon_addr.count(n), taking true branch 2. negative_return_fn: Function this->get_rank(n) returns a negative number. [show details] 3. var_assign: Assigning: signed variable m = get_rank. 4. negative_returns: m is passed to a parameter that cannot be negative. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1188175 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. CID 1188174 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1254381 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member max_fd is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1249635 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member listen_sd is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 717237 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member field default_file_layout.fl_pg_pool is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_stripe_unit is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_stripe_count is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_object_size is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_cas_hash is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_object_stripe_unit is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member field default_file_layout.fl_unused is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1026809 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member want_replica is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member want_xlocked is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member tid is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member pool is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1026812 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member error is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1138594 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member last is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1160851 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member flushed_version is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1188164 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member bits is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1238901 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member ret1 is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ret2 is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ret3 is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1262114 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member map_epoch is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member acks_wanted is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1262115 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR) uninit_member: Non-static class member map_epoch is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member ack_type is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member result is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1297875 (#1 of 1): Arguments in wrong order (SWAPPED_ARGUMENTS) swapped_arguments: The positions of arguments in the call to do_lock_remove do not match the ordering of the parameters: lock_cookie is passed to client lock_client is passed to cookie Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1297874 (#1 of 1): Arguments in wrong order (SWAPPED_ARGUMENTS) swapped_arguments: The positions of arguments in the constructor for CompatSet do not match the ordering of the parameters: feature_incompat_base is passed to _ro_compat feature_ro_compat_base is passed to _incompat Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 717354 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member id is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member type is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member off is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member len is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member lg is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member completion is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1054871 (#1 of 2): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1019635 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1054872 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member curl_inst is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member resp_code is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1188182 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member rcompletion is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1232607 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member m_dump_perf_counters is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_rbd is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_ioctx is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_latency_multiplier is not initialized in this constructor nor in any functions that it calls. uninit_member: Non-static class member m_readonly is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1274321 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member perr is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1274323 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR) uninit_member: Non-static class member perr is not initialized in this constructor nor in any functions that it calls. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1297861 (#1 of 1): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) overflow_before_widen: Potentially overflowing expression this->layout.fl_stripe_count.operator __u32() * this->layout.fl_object_size.operator __u32() with type unsigned int (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type uint64_t (64 bits, unsigned). Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Fix for: CID 1297885 (#1 of 2): Result is not floating-point (UNINTENDED_INTEGER_DIVISION) integer_division: Dividing integer expressions g_conf->mon_pool_quota_warn_threshold and 100, and then converting the integer quotient to type float. Any remainder, or fractional part of the quotient, is ignored. CID 1297885 (#2 of 2): Result is not floating-point (UNINTENDED_INTEGER_DIVISION) integer_division: Dividing integer expressions g_conf->mon_pool_quota_crit_threshold and 100, and then converting the integer quotient to type float. Any remainder, or fractional part of the quotient, is ignored. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Add assert to MonSessionMap::new_session(). Fix for: CID 1128408 (#1 of 1): Dereference before null check (REVERSE_INULL) check_after_deref: Null-checking s suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
CID 1322828 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 28. use_after_free: Using invalidated internal representation of local it. CID 1322827 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 25. use_after_free: Using invalidated internal representation of local it. CID 1322826 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 31. use_after_free: Using invalidated internal representation of local it. CID 1322825 (#1 of 1): Wrapper object use after free (WRAPPER_ESCAPE) 31. use_after_free: Using invalidated internal representation of local it. Signed-off-by: Sage Weil <sage@redhat.com>
CID 1322784 (#1 of 1): Uninitialized scalar variable (UNINIT) 2. uninit_use_in_call: Using uninitialized value coll.removal_seq when calling coll_t. [show details] Signed-off-by: Sage Weil <sage@redhat.com>
CID 1322778 (#1 of 1): Pointer to local outside scope (RETURN_LOCAL) 1. escape_local_addr: Returning, through this->reqid, the address of stack variable _reqid. 2. return: Returning here. Signed-off-by: Sage Weil <sage@redhat.com>
This is from "wip" - Sage Weil <sage@inktank.com> Signed-off-by: Marcus Watts <mwatts@redhat.com>
Proposed as a precursor to conditional XioMessenger changeseet.